Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to read_68() for LTC681x libraries #47

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gudnimg
Copy link

@gudnimg gudnimg commented Nov 18, 2020

I used DC2026C (ATmega328p) and DC2259A (LTC6811) for testing.

  • read_68() is much simpler and is used in more places within the library to reduce repeatability.
  • Function is about 6% faster and code overall is 198 bytes smaller.

Also included:

  • Fix overflow when reading sid with LTC6810.
  • Added PEC mismatch when reading sid with LTC6810.
  • Fix for LTC6813_rdpsb(). Now returns -1 as described in the header file.
  • Removed unused variables in LTC6810_rds_reg()
  • LTC681x_rdaux_reg() now uses read_68()
  • LTC681x_rdcv_reg now uses read_68()
  • LTC681x_rdstat_reg now uses read_68()

gudnimg and others added 3 commits November 18, 2020 01:11
- write_68, read_68, and cmd_68 explicitly define that the command input should be an array with two bytes.
- also fixed a small indent error in LTC681x_stsctrl()
This saves about 130 bytes of memory (at least when I compile for ATmega2560 and ATmega2561). The for loop I removed basically just copies data to rx_data with exactly the same indexes so I don't really see why we need data. Simplifies the code a little bit :)
- Changed cmd[4] to cmd[2] in various places.
@gudnimg
Copy link
Author

gudnimg commented Dec 8, 2020

Another change I might add to this pull request is to not make read_68 return anything at all and not check for a PEC mismatch. Reasoning for that is all the functions that use read_68 they calculate the PEC again for each IC in the daisy chain and update the pec_match variable which read_68 does not.

I tried it and found that it would reduce the program size by an additional 68 bytes. Bringing the total reduction to 198 bytes.

Then we can define the read_68 in a more narrow way: it only reads all the received data and stores it into an array and nothing else.

It would look like this (reduced from 40 lines to 16 lines of code):

void read_68( uint8_t total_ic, // Number of ICs in the system 
				uint8_t tx_cmd[2], // The command to be transmitted 
				uint8_t *rx_data // Data to be read
				)
{
	uint8_t cmd[4];
	uint16_t cmd_pec;
	
	cmd[0] = tx_cmd[0];
	cmd[1] = tx_cmd[1];
	cmd_pec = pec15_calc(2, cmd);
	cmd[2] = (uint8_t)(cmd_pec >> 8);
	cmd[3] = (uint8_t)(cmd_pec);
	
	cs_low(CS_PIN);
	spi_write_read(cmd, 4, rx_data, (NUM_RX_BYT*total_ic));         //Transmits the command and reads the configuration data of all ICs on the daisy chain into rx_data[] array
	cs_high(CS_PIN);                                         
}

I did some testing today with the hardware. Execution time drops from 158μs to around 148μs (This is with only one LTC6811). Not that significant but still an improvement along side the memory reduction. Everything seemed to work as expected.

- Fix for LTC6813_rdpsb(). Now returns proper -1.
- Fix potential overflow in LTC6810_rdsid. It also returns proper -1.
- Removed unused variables in LTC6810_rds_reg()
- LTC681x_rdaux_reg now uses read_68()
- LTC681x_rdcv_reg now uses read_68()
- LTC681x_rdstat_reg now uses read_68()
@gudnimg gudnimg changed the title Remove data from read_68 and use rx_data Improvements to read_68() for LTC681x libraries Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant